Unify month field formatting in test bib files (JabRef/jabref#5116) #5283

sfo · 2019-09-05T12:50:13Z

After some extensive use of grep and sed, this should finally solve #5116.

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

Siedlerchr · 2019-09-05T13:37:58Z

Oh, seems like it broke a lot of tests...

omer-rotem1 · 2019-09-05T20:04:22Z

I tried to look into #5116, as a good first issue. Here's what I did:

Changed the months in the bib files to be in the format oct, not #oct#
Ran tests. 3 RIS tests failed, as they were expecting the month to include the surrounding #s, but they didn't.
Went to RisImporter.java and set the month this way:
if (month.isPresent()) { entry.setField(StandardField.MONTH, month.get().getShortName()); }
But now, other tests have failed - this time, because they actually included the surrounding #s, but were expecting no surroundings #s.

That's why I believe just changing the test bib files won't work, and this requires some changes to the actual code as well.

sfo · 2019-09-06T07:15:16Z

Okay, thanks for your feedback and sorry for the confusion. I will have a look at it.

tobiasdiez · 2019-09-06T21:25:39Z

Thanks for the explanation @omer-rotem1 ! The month should be set via

jabref/src/main/java/org/jabref/model/entry/BibEntry.java

Line 101 in 99a0a7a

public Optional<FieldChange> setMonth(Month parsedMonth) {

omer-rotem1 · 2019-09-07T03:39:58Z

@tobiasdiez Thanks for pointing this out! I agree that if there's one function to set the month it should be used, but as I'm new to JabRef I'm still trying to understand why the month is surrounded by #s in JabRef (see this.)

I'm also interested to know why when JabRef generates the bib file it doesn't serialize the month correctly, but that's probably the bug we're trying to fix here.

omer-rotem1 · 2019-09-07T05:52:59Z

I created a small pull request to fix #5116 (only the 3 tests cases where the bib files were written incorrectly): pull request

tobiasdiez · 2019-09-08T11:01:28Z

I've now merged the PR by @omer-rotem1, so that we get proper attribution for his work. Of course, the other instances of wrong month values found by @sfo should be fixed as well.

(@omer-rotem1 It us usually best-practice in open-source development that you ask before working on the same issue as somebody else. It's just more time economic for everybody involved.)

omer-rotem1 · 2019-09-08T19:01:55Z

@tobiasdiez Thanks for the heads up. Will follow next time.

sfo · 2019-09-09T14:37:10Z

I rebased onto master and fixed the issues with parsers and exporters. Tests are all green again. Looking forward reading your feedback.

tobiasdiez

Thanks for the follow-up. The code look good to me except the changes to the latex parsing/serializing which make me a bit uneasy (see below).

tobiasdiez · 2019-09-09T16:25:57Z

src/main/java/org/jabref/model/entry/Month.java

@@ -175,7 +175,7 @@ public String getShortName() {
     * @return Month in JabRef format
     */
    public String getJabRefFormat() {
-        return String.format("#%s#", shortName);
+        return String.format("%s", shortName);


Please don't change this, but instead use getShortName in the exporters

tobiasdiez · 2019-09-09T16:30:39Z

src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java

@@ -618,7 +618,11 @@ private String parseFieldContent(Field field) throws IOException {
                    throw new IOException("Error in line " + line + " or above: "
                            + "Empty text token.\nThis could be caused " + "by a missing comma between two fields.");
                }
-                value.append('#').append(textToken).append('#');
+                if (field != StandardField.MONTH) {


Although this change makes sense to me and we might change the logic to your version in the future, I would prefer if we don't touch parsing and writing of latex files as part of this PR. There are just too many things that can go wrong and we don't have extensive tests in place to capture them.

Thus, please keep the current behavior here (month = jan is stored as #jan# in BibEntry; moreover, #jan# should be written as month = jan).

I think, this logic is also used to handle string constants (see https://tex.stackexchange.com/a/303489/9075), so we should be very cautious when touching this.

In short: A user should be able to enter a reference to a constant.

BibTex: Journal = IEEE_M_COM <-> entry editor: #IEEE_M_COM#

BibTex: Journal = {IEEE_M_COM} <-> entry editor: IEEE_M_COM

This is a "quick hack" existing since the early days of JabRef. No one came up with another solution. Especially when joining constants with "real" text: BibTex: Journal = IEEE_M_COM # {some more text}

tobiasdiez · 2019-10-03T08:10:07Z

@sfo could you please implement the minor suggestions so that we can merge? Thanks!

koppor · 2019-10-27T14:47:41Z

For reference: The thing with # is described at https://github.com/JabRef/help.jabref.org/blob/gh-pages/en/Strings.md. I also linked it from our entry editor help now.

Did not know that we even support typed String constants. Very nice. I think, this has much potential.

koppor · 2019-10-28T23:07:47Z

I added test cases for month = apr in #5531. master should be merged into this branch. If all test are green, the feedback of tobias should be included. If there are more errors, we really have to think about everything ^^.

koppor · 2019-11-08T13:13:14Z

I went through all the changes and created a follow-up PR at #5578.

30fb68e Create BJEDIS-ABNT-Number (#5255) aafb868 Update geochimica-et-cosmochimica-acta.csl (#5321) 60ba25f british-journal-of-anaesthesia.csl: add comma delimiter between non-sequential citations eg. 1 4 7-9 -> 1, 4, 7-9 (#5313) 67e6564 Reindent/reorder (#5318) c0d2a39 Ruby 3.0.0 (#5309) 76d60ff Update harvard-anglia-ruskin-university.csl (#5310) bc18ac9 Create journal-for-the-study-of-the-new-testament.csl (#5312) aff602c Update journal-of-food-protection.csl (#5315) 4503826 Update muscle-and-nerve.csl (#5317) 3bed58e constant redefinition 4d718a0 update documentaiton link fa99e2f add comma delimiter between succesive numbers d396f8b Allow privileged testing of PRs (#5307) 43b22c7 Update masarykova-univerzita-pravnicka-fakulta.csl, pravnik.csl, iso690-full-note-cs.csl (#5308) 8a31c1e Update copernicus-publications.csl (#5303) 96760bb Update anabases.csl (#5304) 744de6d removed locale (#5300) 7eb0d60 Update aviation-space-and-environmental-medicine.csl (#5297) 2769970 Update ruhr-universitat-bochum-lehrstuhl-fur-industrial-sales-and-ser… (#5298) 51e3f4c Update harvard-university-of-bath.csl (#5299) 5fce84f Create cns-spectrums.csl (#5290) bb8082c Create journal-of-surgical-oncology.csl (#5259) 90c13ae Update ruhr-universitat-bochum-lehrstuhl-fur-industrial-sales-and-ser… (#5288) 4bab1ad Update early-christianity.csl (#5289) 636ba78 Update tatup-zeitschrift-fur-technikfolgenabschatzung-in-theorie-und-… (#5291) b7cc511 Create biotechnologia.csl (#5292) 5bab881 Update journal-of-orthopaedic-trauma.csl (#5287) 5943413 Fix locales (#5285) 302bd65 Update universite-du-quebec-a-montreal-departement-dhistoire.csl (#5286) 860ae48 Add Haaga-Helia University of Applied Sciences Harvard style (#5282) c1c27de Localize Metropolia style title (#5283) 508da89 Fix presentation for Methods of Information in Medicine (#5284) 53e1d0b Create geschichte-und-gesellschaft.csl (#5216) d7ed0cb Create universite-de-geneve-departement-de-francais-moderne.csl (#5212) 80c404b Update journal-of-orthopaedic-trauma.csl (#5281) 20c143a Adding publishers' names (#5280) 6e5cd59 Update sodertorns-hogskola-oxford.csl (#5279) 52f2621 dollar-brace a260294 Create journal-of-microbiology-and-biotechnology.csl (#5277) 1fc979e Create qeios.csl (#5261) 86347b7 GH does this for us -- again, sorry guys b649589 Create experimental-biology-and-medicine.csl (#5276) 12ae0b1 Revert "tell sheldon about the job state" bdcae89 tell sheldon about the job state 1240067 Add Vegetation classification and Survey (#5271) 6f398f0 Major update to Gallia.csl (#5269) 2a74b2c Update filters.yaml (#5273) 20046d2 Update spec_helper.rb (#5272) 2ee0dd8 Create the-sociological-review.csl (#5260) 5b8d09c move filters to inert file to pacify Sheldon (#5268) e5f3315 Localize more language descriptors in style titles (#5270) bfd2942 Localize more language descriptors in style titles (#5267) 35e276f Fix variable used for the label after indication of number of pages (#5240) 60f6371 Create Universidade-do-Estado-do-Rio-de-Janeiro.csl (#5247) d8cc2ae Create the-journal-of-the-acoustical-society-of-america-numeric.csl (#5256) 92259c1 Create journal-of-financial-and-quantitative-analysis.csl (#5264) 6ba8aab Create journal-of-vestibular-research.csl (#5258) 0c88f41 Update european-journal-of-international-law.csl (#5265) cff5abc Put language descriptor within parentheses 4a62709 Update monash-university-harvard.csl (#5253) 64fd1aa Localize more language descriptors in style titles (#5262) f6519cb Localize more language descriptors in style titles (#5257) 170ccae tiny fixes for universitat-basel-iberoromanistik.csl (#5254) b7284c9 Localize more language descriptors in style titles (#5252) f4ef858 Add "Baishideng Publishing Group" dependents (#5251) 266e7c3 Make world-journal-of-hepatology.csl to bpg.csl parent (#5243) 9129098 fix small formatting issues for mclc.csl (#5229) 5d9560b Create crispr-journal.csl (#5249) a217299 Change "Czech" to "Čeština" in titles (#5248) 4fef39a Create journal-of-open-research-software.csl (#5245) 2bff1a6 Change "Dutch" to "Nederlands" in titles (#5242) f28da34 Update spec_helper.rb (#5246) e0e977c Move content from wiki pages to markdown files (#5194) 018304c Update universite-de-montreal-apa.csl (#5239) 3b83e5c Create sodertorns-hogskola-oxford.csl (#5234) 1335378 Stop notifying 8827 port on Zotero servers (#5237) f079b2a Update author-year disambiguation (#5238) 60bb0c9 Update technische-universitat-dresden-medizin.csl (#5236) e374657 Create Leidraad voor juridische auteurs 2019 (Dutch) (#5223) 0450d89 Add new style for U of Mannheim, Germanistische Linguistik (#5228) 81f0689 Create health-sports-rehabilitation-medicine.csl (#5233) c152a44 Update Gemfile.lock (#5235) 748e1eb Update geochimica-et-cosmochimica-acta.csl (#5231) 06b9ce8 Update zeitschrift-fur-theologie-und-philosophie.csl (#5230) e747cb1 haute-ecole-de-gestion-de-geneve: Make polyglot & et al changes 4cfedb7 Create universite-de-sherbrooke-histoire.csl (#5210) a96a61e Update journal-of-glaciology.csl (#5222) c6a94c9 Add Journal of Human Rights (#5227) c5c9c5f Update ruhr-universitat-bochum-lehrstuhl-fur-industrial-sales-and-ser… (#5214) ffb7aa6 Create comparativ.csl (#5215) e07329a Update lancaster-university-harvard.csl (#5220) c075d41 Update mimesis-edizioni.csl (#5219) 502970a Removed space in year only citation (#5218) 13e8c6b Update acta-scientiae-veterinariae.csl (#5209) 0699da6 Remake mammallia.csl for Oct/2020 guidelines. (#5207) b2dd3fd Update journal-of-international-business-studies.csl (#5217) dd52bfe Update quaternaire.csl (#5199) ccb1b0d rebuild webpage and article-journal citations in journal-of-forensic-sciences.csl (#5203) f02f4fb Create pedosphere.csl (#5196) 70dd87a Create open-gender-journal.csl (#5198) d272998 Create the-quarterly-journal-of-economics.csl (#5197) d27cab3 fix locale issues, add cite-locator (#5206) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 30fb68e

sfo added 8 commits September 9, 2019 14:49

Unify month field formatting in test bib files (fixes JabRef#5116)

54d5199

prevent bibtex parser to add # to month

ff2000d

convert medline month to bibtex format

e3f1526

fix jabref format of month

77c25ca

fix month format in IsiImporterTest

b9cfb40

fix month formatting in BibTeXML importer

0c538d1

unify month field formatting in programmatic tests

82e669d

fix BibTeXML exporter to comply with unified month format

dff8170

sfo force-pushed the fix/month_formatting_tests branch from 7f5eecd to dff8170 Compare September 9, 2019 14:29

tobiasdiez requested changes Sep 9, 2019

View reviewed changes

koppor mentioned this pull request Oct 27, 2019

Add tests for month issue and strings (and some minor code impr… #5531

Merged

koppor mentioned this pull request Nov 8, 2019

Unify month field formatting in test bib files #5578

Merged

koppor closed this Nov 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify month field formatting in test bib files (JabRef/jabref#5116) #5283

Unify month field formatting in test bib files (JabRef/jabref#5116) #5283

sfo commented Sep 5, 2019

Siedlerchr commented Sep 5, 2019

omer-rotem1 commented Sep 5, 2019

sfo commented Sep 6, 2019

tobiasdiez commented Sep 6, 2019

omer-rotem1 commented Sep 7, 2019

omer-rotem1 commented Sep 7, 2019

tobiasdiez commented Sep 8, 2019

omer-rotem1 commented Sep 8, 2019

sfo commented Sep 9, 2019

tobiasdiez left a comment

tobiasdiez Sep 9, 2019

tobiasdiez Sep 9, 2019

koppor Oct 6, 2019

tobiasdiez commented Oct 3, 2019

koppor commented Oct 27, 2019

koppor commented Oct 28, 2019

koppor commented Nov 8, 2019

Unify month field formatting in test bib files (JabRef/jabref#5116) #5283

Unify month field formatting in test bib files (JabRef/jabref#5116) #5283

Conversation

sfo commented Sep 5, 2019

Siedlerchr commented Sep 5, 2019

omer-rotem1 commented Sep 5, 2019

sfo commented Sep 6, 2019

tobiasdiez commented Sep 6, 2019

omer-rotem1 commented Sep 7, 2019

omer-rotem1 commented Sep 7, 2019

tobiasdiez commented Sep 8, 2019

omer-rotem1 commented Sep 8, 2019

sfo commented Sep 9, 2019

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Sep 9, 2019

Choose a reason for hiding this comment

tobiasdiez Sep 9, 2019

Choose a reason for hiding this comment

koppor Oct 6, 2019

Choose a reason for hiding this comment

tobiasdiez commented Oct 3, 2019

koppor commented Oct 27, 2019

koppor commented Oct 28, 2019

koppor commented Nov 8, 2019